fix(organizations): membership user_org_unique partial index root-cause#3856
Conversation
MongoDB does not support $ne in partialFilterExpression, so the
schema-declared { userId: { $exists: true, $ne: null } } partial-unique
index always failed server-side; mongoose autoIndex emits that failure
on the model's unlistened 'index' event, so the duplicate-membership
unique guard never materialized on any deployed database. Replace the
spec with { userId: { $type: 'objectId' } } (same intent: null-userId
rows stay excluded), name it user_org_unique, and add the authoritative
migration (duplicate pre-check abort, divergent same-key index drop,
idempotent re-run) with the schema declaring the identical twin.
refs #3841
Prove the user_org_unique index is live at the model level: a second pending row for the same (userId, organizationId) pair rejects with E11000 behind the racy findOne-then-create guards in addMember and createJoinRequest, null-userId rows stay outside the partial index, and syncIndexes() confirms the schema twin matches the migration spec. refs #3841
|
Warning Review limit reached
More reviews will be available in 54 minutes and 36 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughA new MongoDB migration enforces a partial unique index on ChangesMembership unique index enforcement
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3856 +/- ##
==========================================
+ Coverage 91.96% 91.99% +0.03%
==========================================
Files 160 160
Lines 5289 5310 +21
Branches 1698 1708 +10
==========================================
+ Hits 4864 4885 +21
Misses 337 337
Partials 88 88
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Root-cause fix to ensure the intended (userId, organizationId) partial-unique membership constraint actually exists at the database level, restoring a race-proof backstop for duplicate membership creation.
Changes:
- Updates the Membership schema’s compound unique index to use a MongoDB-supported
partialFilterExpression($type: 'objectId') and an explicit stable index name (user_org_unique). - Adds an idempotent migration that pre-checks for duplicates, drops divergent/conflicting indexes, and creates the canonical partial-unique index.
- Adds an integration test suite validating the migration behavior, idempotency, schema/migration alignment (
syncIndexes()), and real E11000 enforcement.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| modules/organizations/models/organizations.membership.model.mongoose.js | Fixes the schema index spec to a MongoDB-supported partial filter and pins the canonical index name for idempotent sync. |
| modules/organizations/migrations/20260612120000-membership-user-org-unique-index.js | Adds an idempotent, safety-checked migration to ensure the canonical partial-unique index exists and is correctly shaped. |
| modules/organizations/tests/organizations.membershipUserOrgUniqueIndex.migration.integration.tests.js | Adds integration coverage for the migration + DB backstop behavior (exact spec, idempotency, abort-on-duplicates, E11000). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@modules/organizations/tests/organizations.membershipUserOrgUniqueIndex.migration.integration.tests.js`:
- Around line 50-53: Add a JSDoc header above the named helper function
findIndex describing its parameter and return value: document the parameter
"name" as a string with `@param` {string} name - index name to find, and document
the return as a Promise resolving to the index object or undefined/null with
`@returns` {Promise<Object|undefined>} (or the actual index type used in the
tests). Keep the description concise and place the JSDoc immediately above the
findIndex function declaration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 74c0f53c-d41f-4b06-8eed-03bf02ff58ed
📒 Files selected for processing (3)
modules/organizations/migrations/20260612120000-membership-user-org-unique-index.jsmodules/organizations/models/organizations.membership.model.mongoose.jsmodules/organizations/tests/organizations.membershipUserOrgUniqueIndex.migration.integration.tests.js
Summary
Root-cause fix for the membership
(userId, organizationId)partial-unique index that never materialized on any deployed DB.partialFilterExpression: { userId: { $exists: true, $ne: null } }. MongoDB does not support$nein apartialFilterExpression, so the index build failed silently — mongooseautoIndexemits the error on the unobserved'index'event, so the duplicate-membership guard had no DB backstop anywhere.partialFilterExpression: { userId: { $type: 'objectId' } }(nulluserIdrows are typenull→ still excluded, preserving the original intent) with the explicit nameuser_org_uniquesoautoIndex/syncIndexesstay idempotent.20260612120000-membership-user-org-unique-index.jsfollows the house check-then-create pattern (pre-check duplicates → abort with a PII-safe ids-only sample; tolerateNamespaceNotFoundon a fresh DB; exact-shapehasIndexcheck; create; idempotent; no-opdown()).Test plan
organizations.membershipUserOrgUniqueIndex.migration.integration.tests.js— index exists with the exact spec afterup(), idempotent re-run, duplicate pre-check abort, and an E11000 DB-backstop (two pending rows same(userId, organizationId)→ second insert rejects). 7/7.Guardrails check
npm run lintcleanCloses #3841
Summary by CodeRabbit
Release Notes
Chores